Fall back to request-token claims for opaque upstream tokens#5147
Open
cjohnhanson wants to merge 2 commits intostacklok:mainfrom
Open
Fall back to request-token claims for opaque upstream tokens#5147cjohnhanson wants to merge 2 commits intostacklok:mainfrom
cjohnhanson wants to merge 2 commits intostacklok:mainfrom
Conversation
VirtualMCPServer (Cedar incoming authz) denied every request when the embedded auth servers upstream provider issues opaque OAuth 2.0 access tokens (Googles ya29.*, GitHubs gho_*). resolveClaims tried to JWT-parse the upstream token unconditionally and returned the parse error verbatim, so every authorization check failed and the gateway skipped every tool. Discriminate by token shape: if the upstream token is not three dot- separated segments it cannot be a JWT, so fall back to identity.Claims (the request-token claims). The embedded auth server already mirrors the upstream OIDC sub, email and name into its issued AS token (see pkg/authserver/server/session/session.go), so policies referencing standard OIDC claims continue to evaluate correctly. JWT-shaped tokens (three segments) that fail to parse still return the error: a tampered or corrupted upstream JWT must not silently degrade to fallback claims. Closes stacklok#5146 Signed-off-by: Cody J. Hanson <cjohnhanson@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ya29.*, GitHub'sgho_*).resolveClaimsJWT-parsed the upstream token unconditionally and returned the parse error verbatim, so every authorization check failed and the gateway skipped every tool.identity.Claims(the request-token claims). The embedded auth server already mirrors the upstream OIDCsub/email/nameinto its issued AS token (seepkg/authserver/server/session/session.go), so policies referencing standard OIDC claims continue to evaluate correctly.looksLikeJWT(tokenStr) = strings.Count(tokenStr, ".") == 2discriminator keeps that boundary explicit.PrimaryUpstreamProvideradded in Fix Cedar upstream-claim evaluation on VirtualMCPServer #5002 (operator side,cmd/thv-operator/pkg/vmcpconfig/converter.go) doesn't expose an opt-out CRD field, so users with opaque-token providers had no in-config workaround. This change unblocks them without changing operator behavior.Closes #5146
Type of change
Test plan
go test ./pkg/authz/...)go vet ./pkg/authz/...gofmt -l pkg/authz/authorizers/cedar/(clean)VirtualMCPServerwithaccounts.google.comupstream, Cedar policy referencingclaim_email. Pre-fix:tools/listreturns empty withAuthorization check failed for tool, skippingper tool. Post-fix: full aggregated tool list returned, single WARN lineupstream token is not a JWT; falling back to request-token claims for Cedar evaluation provider=google.The existing
TestParseUpstreamJWTClaims/opaque_token_returns_errorrow stays unchanged — the helper still returns the same error; only the caller's recovery behavior changes.API Compatibility
v1beta1API. No CRD orConfigOptionsschema change.Changes
pkg/authz/authorizers/cedar/core.goresolveClaimsdiscriminateslooksLikeJWT(tokenStr)before falling back. Opaque (≠3 segments) → fall back toidentity.Claimswith a WARN. JWT-shaped but unparseable → return the original parse error (deny). Adds thelooksLikeJWThelper.pkg/authz/authorizers/cedar/core_test.goupstream_token_opaque_not_parseablerow inTestAuthorizeWithJWTClaims_UpstreamProviderwith three rows: opaque-fallback-denied (policy mismatch via fallback claims), opaque-fallback-permitted (policy matches via fallback claims), and JWT-shaped-but-malformed-still-errors (security-regression guard for tampered JWTs).Does this introduce a user-facing change?
Yes — bug-fix only. Operators with Google (or any opaque-access-token) upstream OIDC provider now receive the aggregated tools list instead of an empty one. No CRD or configuration change required. A WARN log surfaces each fallback so operators can observe the path their identity claims take.
Tampered or corrupted upstream JWTs still hard-deny — there is no behavior change for JWT-access-token providers (Okta, Azure AD with JWT access tokens, etc.).
Special notes for reviewers
A complementary operator-side change — only auto-set
PrimaryUpstreamProviderwhen the user hasn't asked for the explicit fallback — would expose the documented escape hatch via a CRD field, since the docstring onPrimaryUpstreamProvideralready says "When empty, claims from the ToolHive-issued token are used." That route is more invasive (CRD + types + converter + tests). Happy to PR it separately if you'd prefer the opt-in surface over the implicit fallback in this PR. Either approach unblocks the failing case.The fallback could also be argued to belong behind a
ConfigOptions.AllowOpaqueTokenFallbackknob (default false). I left it implicit here because (a) the discriminator preserves the deny for tampered JWTs, which is the security-relevant case, and (b) for opaque-token providers the previous behavior was always-deny — there's no operator who relied on it doing anything useful. Open to adding the gate if you'd prefer the more conservative surface.